Create custom contact properties and verify replication of those properties #244

bhuvana-talend · 2023-11-17T18:21:13Z

Description of change

(https://jira.talendforge.org/browse/TDL-24145)
client.py - Method to create custom contact properties and add it to the contacts
test_hubspot_bookmarks.py - Call the create method in test data setup

Manual QA steps

Risks

Rollback steps

revert this branch

…fied the replication of the same

bhtowles · 2023-12-01T22:45:48Z

Where do these custom properties show up? I would not expect them to necessarily change bookmarks as I assume it is an add on to an existing stream since there is no update in base or streams_to_test. Should we move this to the all fields test and actually test that the custom data is being replicated? The JIRA ticket suggests that we should test that the new data is being replicated.

HarrisonMarcRose

See comments

HarrisonMarcRose · 2023-12-11T20:18:48Z

tests/client.py

+            try:
+                response = self.post(url, current_data)
+                LOGGER.info("response is %s", response)
+            except Exception as e:


This Exception is too broad. We should be a specific with exceptions as possible as this will swallow any exception, some not intended here.

Also, I'm not sure why you would get an exception if data already exists instead of getting a 4xx response code?

yeah, it is a 409, which was causing the test to fail. I'll handle it specifically instead of a broader one.

I think we miscommunicated. I meant in the except Exception as e we should actually change the word Exception to a more specific exception. I don't know what kind it is but based on the 409 maybe it is an RequestException or HTTPError? However, I didn't think a 409 would actually raise an exception. If you are trying to do this until you get a successful status code maybe another way would be better. Let's talk about what you are trying to achieve to determine the best way to go about it.

the post call returns a response code of 409 and in post method the exception is raised. That's why I am handling it here. I can change the exception to a specific one to make it more readable.

HarrisonMarcRose · 2023-12-11T20:24:16Z

tests/client.py

+        property = {
+            "name": "custom_date",
+            "label": "A New Date Custom Property",
+            "description": "A new date property for you",
+            "groupName": "contactinformation",
+            "type": "date",
+            "fieldType": "text",
+            "formField": True,
+            "displayOrder": 9,
+            "options": [
+            ]
+        }
+        data.append(deepcopy(property))
+
+        property = {
+            "name": "custom_datetime",
+            "label": "A New Datetime Custom Property",
+            "description": "A new datetime property for you",
+            "groupName": "contactinformation",
+            "type": "datetime",
+            "fieldType": "text",
+            "formField": True,
+            "displayOrder": 10,
+            "options": [
+            ]
+        }
+        data.append(deepcopy(property))
+
+        property = {
+            "name": "multi_pick",
+            "label": "multi pick",
+            "description": "multi select picklist test",
+            "groupName": "contactinformation",
+            "type": "enumeration",
+            "fieldType": "checkbox",
+            "hidden": False,
+            "options": [
+              {
+                "label": "Option A",
+                "value": "option_a"
+              },
+              {
+                "label": "Option B",
+                "value": "option_b"
+              },
+              {
+                "label": "Option C",
+                "value": "option_c"
+              }
+            ],
+            "formField": True
+        }
+        data.append(deepcopy(property))


It is great that you are creating a variety of data types in the custom fields. Since you went through the pain of creating them we should also use them all when creating a contact - instead of just the first two - to make sure that we support all of the types.

:) was expecting this comment.. The other types seems complicated, I'll need more time to figure out how to do. Can I write a separate card for that ?

bhuvana-talend · 2023-12-11T22:57:24Z

Where do these custom properties show up? I would not expect them to necessarily change bookmarks as I assume it is an add on to an existing stream since there is no update in base or streams_to_test. Should we move this to the all fields test and actually test that the custom data is being replicated? The JIRA ticket suggests that we should test that the new data is being replicated.

@bhtowles , these values are verified in all_fields test. I have verified this by checking the values in the log file manually. Currently, in all fields test we don't check any field specifically. The old contacts won't have this field, the new contacts created after these custom fields are added will have these fields. I can show and explain you. We can move this to all fields test also instead of calling from bookmark test. It is anyway, a one time task to create these fields.

HarrisonMarcRose

Lets talk about these comments

HarrisonMarcRose · 2024-01-04T15:01:30Z

tests/client.py

+        property = {
+            "name": "custom_date",
+            "label": "A New Date Custom Property",
+            "description": "A new date property for you",
+            "groupName": "contactinformation",
+            "type": "date",
+            "fieldType": "text",
+            "formField": True,
+            "displayOrder": 9,
+            "options": [
+            ]
+        }
+        data.append(deepcopy(property))
+
+        property = {
+            "name": "custom_datetime",
+            "label": "A New Datetime Custom Property",
+            "description": "A new datetime property for you",
+            "groupName": "contactinformation",
+            "type": "datetime",
+            "fieldType": "text",
+            "formField": True,
+            "displayOrder": 10,
+            "options": [
+            ]
+        }
+        data.append(deepcopy(property))
+
+        property = {
+            "name": "multi_pick",
+            "label": "multi pick",
+            "description": "multi select picklist test",
+            "groupName": "contactinformation",
+            "type": "enumeration",
+            "fieldType": "checkbox",
+            "hidden": False,
+            "options": [
+              {
+                "label": "Option A",
+                "value": "option_a"
+              },
+              {
+                "label": "Option B",
+                "value": "option_b"
+              },
+              {
+                "label": "Option C",
+                "value": "option_c"
+              }
+            ],
+            "formField": True
+        }
+        data.append(deepcopy(property))


HarrisonMarcRose · 2024-01-04T15:11:41Z

tests/client.py

+            try:
+                response = self.post(url, current_data)
+                LOGGER.info("response is %s", response)
+            except Exception as e:


I think we miscommunicated. I meant in the except Exception as e we should actually change the word Exception to a more specific exception. I don't know what kind it is but based on the 409 maybe it is an RequestException or HTTPError? However, I didn't think a 409 would actually raise an exception. If you are trying to do this until you get a successful status code maybe another way would be better. Let's talk about what you are trying to achieve to determine the best way to go about it.

HarrisonMarcRose · 2024-01-04T15:21:02Z

tests/client.py

+                {
+                    "property": "custom_string",
+                    "value": "custom_string_value"
+                },
+                {
+                    "property": "custom_number",
+                    "value": 1567
+                },


Since we are setting custom properties for every call of create_contacts we need the create_custom_contact_properties to be called prior to calling the create_contacts. I see you are doing this in the test, but what about other tests that would need to add contacts, those would probably break if the custom properties were not present. I believe we should call create_custom_contact_properties in the __init__ of the client so that we call it once when it starts up instead of depending on the tester to know about this or remember to add it in each test.

So, creating the custom contact property is a one time task. After my first successful run of the test, it never creates and gives a 409, that's why I am skipping it. Probably doing it in init is a good idea. Will try that

I think you moved it to setup. I meant the init of the client so anyone using the client makes sure it is in good shape, vs just for your test.

Ok, moved it to init

HarrisonMarcRose · 2024-01-11T15:01:25Z

tests/client.py

+            except Exception as DataAlreadyExistsExcp:
+                LOGGER.info("Data already exists for %s", current_data)
+                if '409' in str(DataAlreadyExistsExcp):
+                    pass
+                else:
+                    response.raise_for_status()


I don't think I"m communicating well enough. doing except Exception is a no-no as this is a very broad exception. We need to figure out the specific exception that is being thrown and catch that. To determine what exception is being thrown you can remove the try/except and run the test to see what exception is raised. Catch that one.

Exception can be used as a wildcard that catches (almost) everything. However, it is good practice to be as specific as possible with the types of exceptions that we intend to handle, and to allow any unexpected exceptions to propagate on.

https://docs.python.org/3/tutorial/errors.html

Sorry Harrison - I think I got it right now :) Now handling it as requests.exceptions.HTTPError

Create custom contact properties, added a few to the contact and veri…

acf4212

…fied the replication of the same

HarrisonMarcRose reviewed Dec 11, 2023

View reviewed changes

bhuvana-talend and others added 3 commits December 12, 2023 02:51

Raise exception if not 409

092bc3a

Merge branch 'master' into tdl-24145-2

f925a65

Merge branch 'master' into tdl-24145-2

53ad094

HarrisonMarcRose reviewed Jan 4, 2024

View reviewed changes

bhuvana-talend added 3 commits January 9, 2024 20:30

Adding one more bad key prefix

1880ea4

Moved create_custom_contact_properties to setup

5b9656d

Changed the exception name and added comments

e37fd59

HarrisonMarcRose reviewed Jan 11, 2024

View reviewed changes

Changed the exception handling

68c46ab

HarrisonMarcRose approved these changes Jan 22, 2024

View reviewed changes

bhuvana-talend and others added 5 commits January 22, 2024 20:10

Resolved conflicts

520643b

Merge branch 'master' into tdl-24145-2

7f42c8e

Resolved conflicts

6103dee

Resolved merge conflicts

1cb5bdb

Merge branch 'master' into tdl-24145-2

100a505

bhuvana-talend merged commit 685a827 into master Jan 25, 2024
15 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create custom contact properties and verify replication of those properties #244

Create custom contact properties and verify replication of those properties #244

bhuvana-talend commented Nov 17, 2023

bhtowles commented Dec 1, 2023 •

edited

Loading

HarrisonMarcRose left a comment

HarrisonMarcRose Dec 11, 2023

bhuvana-talend Dec 11, 2023

HarrisonMarcRose Jan 4, 2024

bhuvana-talend Jan 9, 2024

HarrisonMarcRose Dec 11, 2023

bhuvana-talend Dec 11, 2023

HarrisonMarcRose Jan 4, 2024

bhuvana-talend commented Dec 11, 2023

HarrisonMarcRose left a comment

HarrisonMarcRose Jan 4, 2024

HarrisonMarcRose Jan 4, 2024

HarrisonMarcRose Jan 4, 2024

bhuvana-talend Jan 9, 2024

HarrisonMarcRose Jan 11, 2024

bhuvana-talend Jan 12, 2024

HarrisonMarcRose Jan 11, 2024

HarrisonMarcRose Jan 11, 2024

bhuvana-talend Jan 12, 2024

Create custom contact properties and verify replication of those properties #244

Create custom contact properties and verify replication of those properties #244

Conversation

bhuvana-talend commented Nov 17, 2023

Description of change

Manual QA steps

Risks

Rollback steps

bhtowles commented Dec 1, 2023 • edited Loading

HarrisonMarcRose left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bhuvana-talend commented Dec 11, 2023

HarrisonMarcRose left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bhtowles commented Dec 1, 2023 •

edited

Loading